Skip to content

Conversation

@orware
Copy link
Contributor

@orware orware commented Jul 21, 2016

Pull Request for Issue # .

Summary of Changes

Testing Instructions

Hopefully this is the fastest way to create a new PR with just the changes needed for the JText::_() optimizations from #6788.

Hopefully this is the fastest way to create a new PR with just the changes needed for the JText::_() optimizations from #6788.
{
// Interpret \n and \t characters
$string = str_replace(array('\\\\', '\t', '\n'), array("\\", "\t", "\n"), $string);
if (strpos($string, '\\') !== false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hum you are replaceing 3 different string but only searching the position of one?

@orware
Copy link
Contributor Author

orware commented Jul 21, 2016

Hi Andre,

Yes that is correct...I'm only checking for the presence for at least one backslash character (which would show up in all 3 of the cases being checked against for the str_replace call).

If there is at least one backslash then we need to process the str_replace call because there's a strong chance it'll be needed.

If there isn't at least one backslash then we don't need to process the str_replace call.

I believe that covers things correctly.

Mainly what I observed was that the vast majority of our language strings do not contain any of those 3 characters so running the str_replace call all of the time on every since language string was unnecessary (and there are lots of these calls). The extra if statement helps to avoid them with the faster strpos check.

@andrepereiradasilva
Copy link
Contributor

I have tested this item ✅ successfully on c96e2a8


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11235.

1 similar comment
@ghost
Copy link

ghost commented Jul 22, 2016

I have tested this item ✅ successfully on c96e2a8


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11235.

@brianteeman
Copy link
Contributor

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11235.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 22, 2016
@rdeutz rdeutz added this to the Joomla 3.6.3 milestone Aug 14, 2016
@rdeutz rdeutz merged commit 8877041 into joomla:staging Aug 14, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 14, 2016
izharaazmi added a commit to izharaazmi/joomla-cms that referenced this pull request Aug 15, 2016
* re-arrayhelper-min: (2467 commits)
  Minimize JArrayHelper methods `toInteger`, `pivot`, `arrayUnique` by using Joomla\Utilities\ArrayHelper internally. Leaving (reverting from joomla#7782) other four methods as is for b/c reasons as mentioned in joomla#8455.
  remove platform include (joomla#11615)
  [GitHub Templates] Make headings bigger (joomla#11607)
  [com_contact] Make ACL core.edit.own work (PR for 11466) (joomla#11503)
  Small review on docs & code structure in JModelLegacy library classes (joomla#11057)
  Obviously, this should be an array. (joomla#11610)
  Don't manually import JPlatform anymore (joomla#10841)
  Parse preprocess rules from component routers (joomla#8986)
  Add the correct exception after 11593 merge (was waiting for that merrge) (joomla#11606)
  Add missing clean line after joomla#9277 (joomla#11605)
  Deprecate the _PROFILER global var (joomla#10845)
  Spelling errors (joomla#11604)
  Moved travis javascript bash file to build/travis like joomla#11600 (joomla#11603)
  Regression: Fix edit check in backend articles manager, always denying edit after soft deny (joomla#11511)
  [com_plugins] User not allowed to core.manage? Use 403 php custom exception (instead of a 404 JError) (joomla#11593)
  [com_newsfeeds] Make ACL core.edit.own work (PR for 11466) (joomla#11502)
  $result-variable-undefined-given-default-value (joomla#9277)
  com_banners use exceptions. and not allowed is a 403 (joomla#11418)
  Frontend & plugins using the autoloader (joomla#10882)
  New version of PR 6788 (JText::_() Optimizations) (joomla#11235)
  ...
ggppdk pushed a commit to ggppdk/joomla-cms that referenced this pull request Aug 19, 2016
Hopefully this is the fastest way to create a new PR with just the changes needed for the JText::_() optimizations from joomla#6788.
roland-d pushed a commit to roland-d/joomla-cms that referenced this pull request Sep 11, 2016
Hopefully this is the fastest way to create a new PR with just the changes needed for the JText::_() optimizations from joomla#6788.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants